Fix rate limiting on scan - #6
Conversation
bennibu
left a comment
bennibu
left a comment
There was a problem hiding this comment.
Moin Stefan, du hast ja einiges umgebaut hier. Als zentralen Unterschied zu vorher sehe ich, dass der scanner bei einem 520 oder nach 5 Versuchen austeigt. Vorher hat er bei einem 520 es trotzdem noch 5 mal versucht. Inwieweit soll das das Problem lösen?
|
@bennibu an der Stelle hab ich eigentlich nur die Logik umgedreht, weil sich das besser im Code gemacht hat (bisher: bei 520 und weniger als 5 weitermachen; jetzt: bei nicht 520 oder mehr als 5 aussteigen). Der relevante Teil ist, dass es zwei API-Calls gibt und bisher nur der erste API-Call abgesichert war. Mit dem neuen Code sind beide API-Calls abgesichert: quayio-scanner/lib/quayio/scanner/image.rb Lines 72 to 73 in 5c6ac6a |
|
@stefan-as Kompliziert, aber ich verstehe jetzt. Ist auf jeden Fall deutlich lesbarer als vorher. Danke. |
There was a problem hiding this comment.
VIelen Dank! Habe wie folgt gereviewed:
Die Code-Vorschläge sind als Anmerkungen, nicht als Wünsche gemeint.
Lass uns über die konzeptionellen Fragen gerne nochmal sprechen.
Im folgenden noch eine Sache, die mir insgesamt mit dem Repo aufgefallen ist:
Review zu Code außerhalb der Änderungen diesen PRs
Anmerkung: Imho ist es Convention im "Root"-File eines Gems alle Dateien in (Sub-)Modulen des Gems (quayio/scanner/image, quayio/scanner/repository, ...) einzusammeln, und in den einzelnen Files der Module nur noch externe Dependencies (docker, rest-client, ...) zu requiren. Z.B.:
# lib/quayio/scanner.rb
require 'quayio/scanner/check'
require 'quayio/scanner/image'
require 'quayio/scanner/repository'
require 'quayio/scanner/version'
module Quayio
module Scanner
end
endSiehe
quayio-scanner/lib/quayio/scanner.rb
Lines 1 to 7 in 16e77a4
Ein Beleg: https://ryanlue.com/posts/2017-03-03-how-to-structure-a-library#rule-3-one-file-to-rule-them-all
jonathanschlue-as
left a comment
jonathanschlue-as
left a comment
There was a problem hiding this comment.
Noch ein paar Ruby-Dinge
3 participants
Der Check ruft zweimal die QuayIO-API auf. Der Schutz gegen Rate-Limiting war aber nur an einem Aufruf implementiert. Ich habe den Moment genutzt, um die API-Calls zu generalisieren und in eine eigene Klasse zu kapseln.